Skip to content

Add Branding Manager, add GitHub token#1289

Merged
MarkKoz merged 32 commits into
python-discord:masterfrom
ks129:branding-manager
Jan 19, 2021
Merged

Add Branding Manager, add GitHub token#1289
MarkKoz merged 32 commits into
python-discord:masterfrom
ks129:branding-manager

Conversation

@ks129
Copy link
Copy Markdown
Contributor

@ks129 ks129 commented Nov 17, 2020

Branding manager move got approved in #dev-contrib

Added GH token from env variable GITHUB_TOKEN. Branding manager and PEPs use this now.

Moved branding manager from SeasonalBot (Cobra) to Python.

@ks129 ks129 requested a review from a team as a code owner November 17, 2020 16:46
@ks129 ks129 requested review from MrHemlock and jb3 and removed request for a team November 17, 2020 16:46
@jb3 jb3 removed their request for review November 28, 2020 02:46
@ks129 ks129 requested a review from Akarys42 as a code owner December 17, 2020 15:58
@ghost ghost added the needs 2 approvals label Dec 17, 2020
@ks129 ks129 added the a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) label Dec 17, 2020
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 17, 2020

Coverage Status

Coverage decreased (-0.5%) to 56.696% when pulling b14933d on ks129:branding-manager into 1fc0789 on python-discord:master.

Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've done an absolutely great job of seamlessly transferring this feature from sir-lancebot to this bot.

I've reviewed this as thoroughly as I could and I didn't find any conflicts or dead functions/methods/classes.

After the conflicts are resolved, this PR should be good to go.

@Den4200
Copy link
Copy Markdown
Member

Den4200 commented Jan 2, 2021

Could you please resolve these conflicts @ks129?

@ks129
Copy link
Copy Markdown
Contributor Author

ks129 commented Jan 3, 2021

@Den4200 Conflicts resolved.

Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see seasons.py, branding.py, and the feature-specific constants all under one sub-package.

Comment thread bot/exts/backend/branding.py Outdated
Comment thread bot/seasons.py Outdated
Den4200
Den4200 previously requested changes Jan 6, 2021
Comment thread bot/errors.py Outdated
@ks129 ks129 requested review from Den4200 and MarkKoz January 6, 2021 07:48
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a little overkill to define an error module for a single error, but it's honestly fine so don't worry about changing it.

Comment thread bot/exts/backend/branding/_cog.py
@ks129 ks129 requested a review from MarkKoz January 16, 2021 18:19
@MarkKoz MarkKoz merged commit cbeb6eb into python-discord:master Jan 19, 2021
@ks129 ks129 deleted the branding-manager branch January 19, 2021 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants